Skip to content

Conversation

@JDevlieghere
Copy link
Member

Completes the work started in #128262. This PR removes the old way of register request handlers with callbacks. Builds on top of #128551.

@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Completes the work started in #128262. This PR removes the old way of register request handlers with callbacks. Builds on top of #128551.


Patch is 234.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128553.diff

29 Files Affected:

  • (modified) lldb/tools/lldb-dap/CMakeLists.txt (+23)
  • (modified) lldb/tools/lldb-dap/DAP.cpp (+2-15)
  • (modified) lldb/tools/lldb-dap/DAP.h (+2-17)
  • (added) lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp (+80)
  • (added) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp (+190)
  • (added) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+222)
  • (added) lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp (+160)
  • (added) lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp (+58)
  • (added) lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp (+79)
  • (added) lldb/tools/lldb-dap/Handler/PauseRequestHandler.cpp (+60)
  • (added) lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp (+139)
  • (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+62)
  • (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+176-1)
  • (added) lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp (+106)
  • (added) lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp (+182)
  • (added) lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp (+114)
  • (added) lldb/tools/lldb-dap/Handler/SetExceptionBreakpointsRequestHandler.cpp (+93)
  • (added) lldb/tools/lldb-dap/Handler/SetFunctionBreakpointsRequestHandler.cpp (+139)
  • (added) lldb/tools/lldb-dap/Handler/SetInstructionBreakpointsRequestHandler.cpp (+249)
  • (added) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+177)
  • (added) lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp (+82)
  • (added) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+197)
  • (added) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+96)
  • (added) lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp (+149)
  • (added) lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp (+68)
  • (added) lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp (+31)
  • (added) lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp (+71)
  • (added) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+217)
  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+25-2678)
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 73762af5c2fd7..804dd8e4cc2a0 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -39,16 +39,39 @@ add_lldb_tool(lldb-dap
 
   Handler/AttachRequestHandler.cpp
   Handler/BreakpointLocationsHandler.cpp
+  Handler/CompileUnitsRequestHandler.cpp
   Handler/CompletionsHandler.cpp
   Handler/ConfigurationDoneRequestHandler.cpp
   Handler/ContinueRequestHandler.cpp
+  Handler/DataBreakpointInfoRequestHandler.cpp
+  Handler/DisassembleRequestHandler.cpp
   Handler/DisconnectRequestHandler.cpp
   Handler/EvaluateRequestHandler.cpp
   Handler/ExceptionInfoRequestHandler.cpp
   Handler/InitializeRequestHandler.cpp
   Handler/LaunchRequestHandler.cpp
+  Handler/LocationsRequestHandler.cpp
+  Handler/ModulesRequestHandler.cpp
+  Handler/NextRequestHandler.cpp
+  Handler/PauseRequestHandler.cpp
+  Handler/ReadMemoryRequestHandler.cpp
   Handler/RequestHandler.cpp
   Handler/RestartRequestHandler.cpp
+  Handler/ScopesRequestHandler.cpp
+  Handler/SetBreakpointsRequestHandler.cpp
+  Handler/SetDataBreakpointsRequestHandler.cpp
+  Handler/SetExceptionBreakpointsRequestHandler.cpp
+  Handler/SetFunctionBreakpointsRequestHandler.cpp
+  Handler/SetInstructionBreakpointsRequestHandler.cpp
+  Handler/SetVariableRequestHandler.cpp
+  Handler/SourceRequestHandler.cpp
+  Handler/StackTraceRequestHandler.cpp
+  Handler/StepInRequestHandler.cpp
+  Handler/StepInTargetsRequestHandler.cpp
+  Handler/StepOutRequestHandler.cpp
+  Handler/TestGetTargetBreakpointsRequestHandler.cpp
+  Handler/ThreadsRequestHandler.cpp
+  Handler/VariablesRequestHandler.cpp
 
   LINK_LIBS
     liblldb
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 9b22b60a68d94..d5dd0304f7221 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -758,20 +758,12 @@ bool DAP::HandleObject(const llvm::json::Object &object) {
   if (packet_type == "request") {
     const auto command = GetString(object, "command");
 
-    // Try the new request handler first.
-    auto new_handler_pos = new_request_handlers.find(command);
-    if (new_handler_pos != new_request_handlers.end()) {
+    auto new_handler_pos = request_handlers.find(command);
+    if (new_handler_pos != request_handlers.end()) {
       (*new_handler_pos->second)(object);
       return true; // Success
     }
 
-    // FIXME: Remove request_handlers once everything has been migrated.
-    auto handler_pos = request_handlers.find(command);
-    if (handler_pos != request_handlers.end()) {
-      handler_pos->second(*this, object);
-      return true; // Success
-    }
-
     if (log)
       *log << "error: unhandled command \"" << command.data() << "\""
            << std::endl;
@@ -900,11 +892,6 @@ void DAP::SendReverseRequest(llvm::StringRef command,
   });
 }
 
-void DAP::RegisterRequestCallback(std::string request,
-                                  RequestCallback callback) {
-  request_handlers[request] = callback;
-}
-
 lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
   lldb::SBError error;
   lldb::SBProcess process = target.GetProcess();
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 18de39838f218..ddda8603c189f 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -68,7 +68,6 @@ enum DAPBroadcasterBits {
   eBroadcastBitStopProgressThread = 1u << 1
 };
 
-typedef void (*RequestCallback)(DAP &dap, const llvm::json::Object &command);
 typedef void (*ResponseCallback)(llvm::Expected<llvm::json::Value> value);
 
 enum class PacketStatus {
@@ -186,8 +185,7 @@ struct DAP {
   // the old process here so we can detect this case and keep running.
   lldb::pid_t restarting_process_id;
   bool configuration_done_sent;
-  std::map<std::string, RequestCallback, std::less<>> request_handlers;
-  llvm::StringMap<std::unique_ptr<RequestHandler>> new_request_handlers;
+  llvm::StringMap<std::unique_ptr<RequestHandler>> request_handlers;
   bool waiting_for_run_in_terminal;
   ProgressEventReporter progress_event_reporter;
   // Keep track of the last stop thread index IDs as threads won't go away
@@ -305,8 +303,6 @@ struct DAP {
   /// listeing for its breakpoint events.
   void SetTarget(const lldb::SBTarget target);
 
-  const std::map<std::string, RequestCallback> &GetRequestHandlers();
-
   PacketStatus GetNextObject(llvm::json::Object &object);
   bool HandleObject(const llvm::json::Object &object);
 
@@ -334,20 +330,9 @@ struct DAP {
   void SendReverseRequest(llvm::StringRef command, llvm::json::Value arguments,
                           ResponseCallback callback);
 
-  /// Registers a callback handler for a Debug Adapter Protocol request
-  ///
-  /// \param[in] request
-  ///     The name of the request following the Debug Adapter Protocol
-  ///     specification.
-  ///
-  /// \param[in] callback
-  ///     The callback to execute when the given request is triggered by the
-  ///     IDE.
-  void RegisterRequestCallback(std::string request, RequestCallback callback);
-
   /// Registers a request handler.
   template <typename Handler> void RegisterRequest() {
-    new_request_handlers[Handler::getCommand()] =
+    request_handlers[Handler::getCommand()] =
         std::make_unique<Handler>(*this);
   }
 
diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
new file mode 100644
index 0000000000000..c541d1cd039c8
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp
@@ -0,0 +1,80 @@
+//===-- CompileUnitsRequestHandler.cpp ------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAP.h"
+#include "EventHelper.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+
+namespace lldb_dap {
+
+// "compileUnitsRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Compile Unit request; value of command field is
+//                     'compileUnits'.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "compileUnits" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/compileUnitRequestArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments" ]
+//   }]
+// },
+// "compileUnitsRequestArguments": {
+//   "type": "object",
+//   "description": "Arguments for 'compileUnits' request.",
+//   "properties": {
+//     "moduleId": {
+//       "type": "string",
+//       "description": "The ID of the module."
+//     }
+//   },
+//   "required": [ "moduleId" ]
+// },
+// "compileUnitsResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to 'compileUnits' request.",
+//     "properties": {
+//       "body": {
+//         "description": "Response to 'compileUnits' request. Array of
+//                         paths of compile units."
+//       }
+//     }
+//   }]
+// }
+void CompileUnitsRequestHandler::operator()(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  llvm::json::Array units;
+  const auto *arguments = request.getObject("arguments");
+  std::string module_id = std::string(GetString(arguments, "moduleId"));
+  int num_modules = dap.target.GetNumModules();
+  for (int i = 0; i < num_modules; i++) {
+    auto curr_module = dap.target.GetModuleAtIndex(i);
+    if (module_id == curr_module.GetUUIDString()) {
+      int num_units = curr_module.GetNumCompileUnits();
+      for (int j = 0; j < num_units; j++) {
+        auto curr_unit = curr_module.GetCompileUnitAtIndex(j);
+        units.emplace_back(CreateCompileUnit(curr_unit));
+      }
+      body.try_emplace("compileUnits", std::move(units));
+      break;
+    }
+  }
+  response.try_emplace("body", std::move(body));
+  dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
new file mode 100644
index 0000000000000..519a9c728e4b3
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp
@@ -0,0 +1,190 @@
+//===-- DataBreakpointInfoRequestHandler.cpp ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAP.h"
+#include "EventHelper.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace lldb_dap {
+
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Obtains information on a possible data breakpoint that
+//     could be set on an expression or variable.\nClients should only call this
+//     request if the corresponding capability `supportsDataBreakpoints` is
+//     true.", "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "dataBreakpointInfo" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/DataBreakpointInfoArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+//     "variablesReference": {
+//       "type": "integer",
+//       "description": "Reference to the variable container if the data
+//       breakpoint is requested for a child of the container. The
+//       `variablesReference` must have been obtained in the current suspended
+//       state. See 'Lifetime of Object References' in the Overview section for
+//       details."
+//     },
+//     "name": {
+//       "type": "string",
+//       "description": "The name of the variable's child to obtain data
+//       breakpoint information for.\nIf `variablesReference` isn't specified,
+//       this can be an expression."
+//     },
+//     "frameId": {
+//       "type": "integer",
+//       "description": "When `name` is an expression, evaluate it in the scope
+//       of this stack frame. If not specified, the expression is evaluated in
+//       the global scope. When `variablesReference` is specified, this property
+//       has no effect."
+//     }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `dataBreakpointInfo` request.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "dataId": {
+//             "type": [ "string", "null" ],
+//             "description": "An identifier for the data on which a data
+//             breakpoint can be registered with the `setDataBreakpoints`
+//             request or null if no data breakpoint is available. If a
+//             `variablesReference` or `frameId` is passed, the `dataId` is
+//             valid in the current suspended state, otherwise it's valid
+//             indefinitely. See 'Lifetime of Object References' in the Overview
+//             section for details. Breakpoints set using the `dataId` in the
+//             `setDataBreakpoints` request may outlive the lifetime of the
+//             associated `dataId`."
+//           },
+//           "description": {
+//             "type": "string",
+//             "description": "UI string that describes on what data the
+//             breakpoint is set on or why a data breakpoint is not available."
+//           },
+//           "accessTypes": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/DataBreakpointAccessType"
+//             },
+//             "description": "Attribute lists the available access types for a
+//             potential data breakpoint. A UI client could surface this
+//             information."
+//           },
+//           "canPersist": {
+//             "type": "boolean",
+//             "description": "Attribute indicates that a potential data
+//             breakpoint could be persisted across sessions."
+//           }
+//         },
+//         "required": [ "dataId", "description" ]
+//       }
+//     },
+//     "required": [ "body" ]
+//   }]
+// }
+void DataBreakpointInfoRequestHandler::operator()(
+    const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  llvm::json::Object body;
+  lldb::SBError error;
+  llvm::json::Array accessTypes{"read", "write", "readWrite"};
+  const auto *arguments = request.getObject("arguments");
+  const auto variablesReference =
+      GetUnsigned(arguments, "variablesReference", 0);
+  llvm::StringRef name = GetString(arguments, "name");
+  lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
+  lldb::SBValue variable = FindVariable(variablesReference, name);
+  std::string addr, size;
+
+  if (variable.IsValid()) {
+    lldb::addr_t load_addr = variable.GetLoadAddress();
+    size_t byte_size = variable.GetByteSize();
+    if (load_addr == LLDB_INVALID_ADDRESS) {
+      body.try_emplace("dataId", nullptr);
+      body.try_emplace("description",
+                       "does not exist in memory, its location is " +
+                           std::string(variable.GetLocation()));
+    } else if (byte_size == 0) {
+      body.try_emplace("dataId", nullptr);
+      body.try_emplace("description", "variable size is 0");
+    } else {
+      addr = llvm::utohexstr(load_addr);
+      size = llvm::utostr(byte_size);
+    }
+  } else if (variablesReference == 0 && frame.IsValid()) {
+    lldb::SBValue value = frame.EvaluateExpression(name.data());
+    if (value.GetError().Fail()) {
+      lldb::SBError error = value.GetError();
+      const char *error_cstr = error.GetCString();
+      body.try_emplace("dataId", nullptr);
+      body.try_emplace("description", error_cstr && error_cstr[0]
+                                          ? std::string(error_cstr)
+                                          : "evaluation failed");
+    } else {
+      uint64_t load_addr = value.GetValueAsUnsigned();
+      lldb::SBData data = value.GetPointeeData();
+      if (data.IsValid()) {
+        size = llvm::utostr(data.GetByteSize());
+        addr = llvm::utohexstr(load_addr);
+        lldb::SBMemoryRegionInfo region;
+        lldb::SBError err =
+            dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
+        // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
+        // request if SBProcess::GetMemoryRegionInfo returns error.
+        if (err.Success()) {
+          if (!(region.IsReadable() || region.IsWritable())) {
+            body.try_emplace("dataId", nullptr);
+            body.try_emplace("description",
+                             "memory region for address " + addr +
+                                 " has no read or write permissions");
+          }
+        }
+      } else {
+        body.try_emplace("dataId", nullptr);
+        body.try_emplace("description",
+                         "unable to get byte size for expression: " +
+                             name.str());
+      }
+    }
+  } else {
+    body.try_emplace("dataId", nullptr);
+    body.try_emplace("description", "variable not found: " + name.str());
+  }
+
+  if (!body.getObject("dataId")) {
+    body.try_emplace("dataId", addr + "/" + size);
+    body.try_emplace("accessTypes", std::move(accessTypes));
+    body.try_emplace("description",
+                     size + " bytes at " + addr + " " + name.str());
+  }
+  response.try_emplace("body", std::move(body));
+  dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
new file mode 100644
index 0000000000000..6d25ef0fc5d74
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -0,0 +1,222 @@
+//===-- DisassembleRequestHandler.cpp -------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAP.h"
+#include "EventHelper.h"
+#include "JSONUtils.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBInstruction.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace lldb_dap {
+
+// "DisassembleRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Disassembles code stored at the provided
+//     location.\nClients should only call this request if the corresponding
+//     capability `supportsDisassembleRequest` is true.", "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "disassemble" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/DisassembleArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments" ]
+//   }]
+// },
+// "DisassembleArguments": {
+//   "type": "object",
+//   "description": "Arguments for `disassemble` request.",
+//   "properties": {
+//     "memoryReference": {
+//       "type": "string",
+//       "description": "Memory reference to the base location containing the
+//       instructions to disassemble."
+//     },
+//     "offset": {
+//       "type": "integer",
+//       "description": "Offset (in bytes) to be applied to the reference
+//       location before disassembling. Can be negative."
+//     },
+//     "instructionOffset": {
+//       "type": "integer",
+//       "description": "Offset (in instructions) to be applied after the byte
+//       offset (if any) before disassembling. Can be negative."
+//     },
+//     "instructionCount": {
+//       "type": "integer",
+//       "description": "Number of instructions to disassemble starting at the
+//       specified location and offset.\nAn adapter must return exactly this
+//       number of instructions - any unavailable instructions should be
+//       replaced with an implementation-defined 'invalid instruction' value."
+//     },
+//     "resolveSymbols": {
+//       "type": "boolean",
+//       "description": "If true, the adapter should attempt to resolve memory
+//       addresses and other values to symbolic names."
+//     }
+//   },
+//   "required": [ "memoryReference", "instructionCount" ]
+// },
+// "DisassembleResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `disassemble` request.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "instructions": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/DisassembledInstruction"
+//             },
+//             "description": "The list of disassembled instructions."
+//           }
+//         },
+//         "required": [ "instructions" ]
+//       }
+//     }
+//   }]
+// }
+void DisassembleRequestHandler::operator()(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  auto *arguments = request.getObject("arguments");
+
+  llvm::StringRef memoryReference = GetString(arguments, "memoryReference");
+  auto addr_opt = DecodeMemoryReference(memoryReference);
+  if (!addr_opt.has_value()) {
+    response["success"] = false;
+    response["message"] =
+        "Malformed memory reference: " + memoryR...
[truncated]

@github-actions
Copy link

github-actions bot commented Feb 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@ashgti ashgti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM


virtual ~RequestHandler() = default;

virtual void operator()(const llvm::json::Object &request) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also mark this const? I don't think any of the request handlers are using any state at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I was going to it in a separate commit, but I can include it here.

This removes the old way of register request handlers with callbacks.
@JDevlieghere JDevlieghere force-pushed the finish-request-handlers branch from 76c1fd8 to 611d0c9 Compare February 24, 2025 22:06
@JDevlieghere JDevlieghere merged commit 911e94c into llvm:main Feb 24, 2025
10 checks passed
@JDevlieghere JDevlieghere deleted the finish-request-handlers branch February 24, 2025 22:46
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
Completes the work started in llvm#128262. This PR removes the
old way of register request handlers with callbacks and makes
the operator const.
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jul 14, 2025
Completes the work started in llvm#128262. This PR removes the
old way of register request handlers with callbacks and makes
the operator const.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants